Skip to content

Fix MongoDB query safety, auth bypass, and devicestatus refactor#1

Open
sergev-art wants to merge 1 commit intomasterfrom
fix/mongodb-query-safety-and-auth-bypass
Open

Fix MongoDB query safety, auth bypass, and devicestatus refactor#1
sergev-art wants to merge 1 commit intomasterfrom
fix/mongodb-query-safety-and-auth-bypass

Conversation

@sergev-art
Copy link
Owner

Summary

Addresses three safety-critical issues identified in upstream review (nightscout#8447, nightscout#8421):

  • ObjectID coercion guard (lib/server/query.js): updateIdQuery crashed on UUID-format _id values. Now validates 24-char hex pattern before coercing to ObjectID.
  • Auth failure delay bypass (lib/authorization/delaylist.js): authFailDelay = 0 was silently accepted via || 5000 fallback, eliminating brute-force protection. Replaced with explicit positive-finite guard.
  • Device status refactor (lib/server/devicestatus.js): Replaced serial insertOne loop with insertMany. Added truncatePredictions(obj, maxSize) to cap prediction arrays at 288 points (24h × 5min), preventing MongoDB 16MB document limit breach. Modernized remove to deleteMany.
  • Boot sequence update (lib/server/bootevent.js): Updated call signature to match refactored devicestatus module.

Design Control Impact

This PR affects the following design control areas:

Modified Requirements

  • Query handling: ObjectID validation rules (new regex guard for 24-char hex)
  • Authentication: Auth failure delay specification (explicit positive-finite guard, no longer accepts 0)
  • Device status storage: Batch insertion, prediction truncation at 288 data points

New Test Cases Needed

  • ObjectID coercion: valid hex → ObjectID, UUID → passthrough (2 tests included in tests/query.test.js)
  • truncatePredictions boundary conditions (at/below/above 288 points)
  • insertMany batch behavior vs. former serial insertOne
  • authFailDelay with values: 0, positive integer, missing/undefined, non-numeric

Test Modifications

  • Existing devicestatus tests may need signature updates (storage(collection, ctx)storage(env, ctx))

Risk Items

  • Security: Auth delay bypass allowed brute-force attacks (URGENT)
  • Reliability: Server crash on UUID-format IDs from certain data sources
  • Data integrity: Unbounded prediction arrays could exceed MongoDB 16MB document limit

Files Changed (5)

File Change Lines
lib/server/query.js ObjectID coercion guard +6/-2
lib/authorization/delaylist.js Auth delay bypass fix +3/-1
lib/server/devicestatus.js Refactor + truncatePredictions +95/-38
lib/server/bootevent.js Signature update +1/-1
tests/query.test.js New ObjectID tests +14/-0

Co-authored-by: bewest <394179+bewest@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants